Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add support for exemplars #432

Closed
wants to merge 2 commits into from

Conversation

samarara
Copy link

@samarara samarara commented Apr 1, 2021

With exemplar support coming to Prometheus version 2.26.0 I thought I could start on this. Also addresses #410.

Looking for some direction to see if this the right approach to take. I started with Counter and I'm going to add another function to registry to call depending on register.contentType

@samarara samarara force-pushed the feat/exemplar-support branch 2 times, most recently from e6f607b to 55fe377 Compare April 3, 2021 18:53
@SimenB SimenB requested review from zbjornson and siimon April 5, 2021 18:13
@samarara samarara force-pushed the feat/exemplar-support branch from 55fe377 to 198d9d7 Compare April 8, 2021 00:53
@samarara samarara force-pushed the feat/exemplar-support branch from 198d9d7 to f5d33ed Compare April 8, 2021 01:03
@samarara
Copy link
Author

samarara commented Apr 8, 2021

@siimon @zbjornson I implemented this similar to labels. It publishes metrics in a similar fashion as prometheus uses to parse them. If content-type is application/openmetrics-text the metrics are published with exemplars and the OpenMetrics parser is used in prometheus to parse them.

Let me know if the direction I'm taking is okay. So far it satisfies everything in the OpenMetrics exemplar spec except for the optional timestamp. I'll add that shortly.

@samarara samarara marked this pull request as ready for review April 8, 2021 01:13
@samarara
Copy link
Author

@zbjornson @siimon any advice on this?

@AaronFriel
Copy link

@zbjornson @siimon would you be OK with someone else reviewing this MR?

@shyimo
Copy link

shyimo commented Sep 20, 2021

@samarara thanks for this MR. i would love to help if you need.
We are using jaeger tracing system and really looking forward for this node.js client support

@zbjornson
Copy link
Collaborator

Sorry for the delay. I need to read up on exemplars and OpenMetrics to review this, but am in favor of adding support. If someone else who's familiar with these concepts wants to help review, that'd be much appreciated!

It would also be helpful if someone can provide links to docs or code for the equivalent functionality in the other Prometheus client libraries (go/java/.NET/etc.).

@samarara
Copy link
Author

This MR is a bit out dated as the counter logic has changed since I authored this. Any help would be greatly appreciated. Here is the spec:
https://github.com/OpenObservability/OpenMetrics/blob/main/specification/OpenMetrics.md#exemplars

For the Java client it's implemented for both counters and histograms: https://github.com/OpenObservability/OpenMetrics/blob/main/specification/OpenMetrics.md#exemplars-1
implementation: https://github.com/prometheus/client_java/blob/7a29af04821cd51592c45ce4682c3132372f4c5d/simpleclient/src/main/java/io/prometheus/client/exemplars/Exemplar.java#L10

@SimenB
Copy link
Collaborator

SimenB commented Oct 2, 2023

This landed in #544 I believe - sorry about the radio silence!

@SimenB SimenB closed this Oct 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants